tools,makefile: fix file open mode and cleanup tap file #2837
Conversation
|
LGTM |
1 similar comment
|
LGTM |
|
...with a teeny nit about the commit logs: s/Open/open/ and s/Makefile/build/ for consistency. |
By default the logfile is opened in append mode. This commit makes sure that the file is opened in write-binary mode, so that the file will be created if it doesn't exist or overwrite if it exists. Fixes: nodejs#2834
Make `make clean` cleanup the generated tap file as well. Fixes: nodejs#2834
thefourtheye
force-pushed
the
fix-for-2834
branch
from
f933250
to
89d5237
Compare
|
lgtm, thanks for doing this @thefourtheye |
|
I wonder: does appending to a .tap file ever make sense? If not, it would be better to have the test runner delete the file (instead of make test-ci). |
|
@orangemocha If test-runner deletes the file, then we will not get to see the TAP results, right? |
|
Maybe delete before running the tests? |
|
@evanlucas This patch does something similar. Opens the file in write mode, which truncates the file if it exists. |
|
@thefourtheye : sorry, I didn't catch that before. Your first commit already does what I was suggesting :) LGTM |
By default the logfile is opened in append mode. This commit makes sure that the file is opened in write-binary mode, so that the file will be created if it doesn't exist or overwrite if it exists. Fixes: #2834 PR-URL: #2837 Reviewed-By: Johan Bergström <bugs@bergstroem.nu> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Rod Vagg <rod@vagg.org> Reviewed-By: Alexis Campailla <orangemocha@nodejs.org>
By default the logfile is opened in append mode. This commit makes sure that the file is opened in write-binary mode, so that the file will be created if it doesn't exist or overwrite if it exists. Fixes: #2834 PR-URL: #2837 Reviewed-By: Johan Bergström <bugs@bergstroem.nu> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Rod Vagg <rod@vagg.org> Reviewed-By: Alexis Campailla <orangemocha@nodejs.org>
|
@thefourtheye I'm just noticing that these two commits made it in with |
|
@rvagg Ah, sorry about that. I just used that GitHub format. I'll keep it in mind for the next time. |
|
@thefourtheye downside being very hard to click in terminal :) |
By default the logfile is opened in append mode. This commit makes sure that the file is opened in write-binary mode, so that the file will be created if it doesn't exist or overwrite if it exists. Fixes: #2834 PR-URL: #2837 Reviewed-By: Johan Bergström <bugs@bergstroem.nu> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Rod Vagg <rod@vagg.org> Reviewed-By: Alexis Campailla <orangemocha@nodejs.org>
1st commit:
By default the logfile is opened in append mode. This commit makes sure
that the file is opened in write-binary mode, so that the file will be
created if it doesn't exist or overwrite if it exists.
2nd commit:
Make
make cleancleanup the generated tap file as well.Fixes: #2834
cc @nodejs/build